fix(common): enforce the full SASL contract in streaming.env - #1805
Merged
Conversation
lib-streaming fails closed at bootstrap on an incomplete SASL config, but the chart only validated the password (and username, in the Secret which is skipped for useExistingSecret). Validate the whole contract in streaming.env — the ConfigMap, which ALWAYS renders, so it covers the external-Secret case too. When a mechanism is set: - it must be one of PLAIN, SCRAM-SHA-256, SCRAM-SHA-512 (reject e.g. GSSAPI); - a username is required (configmap.STREAMING_SASL_USERNAME -> global), including with an existing Secret (username is a ConfigMap value, known at render); - TLS must be on (STREAMING_TLS_ENABLED=true) unless plaintext SASL is explicitly opted into (STREAMING_SASL_ALLOW_PLAINTEXT=true). Validated: TLS-off fails, GSSAPI fails, missing username fails, valid config + plaintext opt-in render; no SASL is unaffected.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe streaming template now validates configured SASL mechanisms, requires a username, and requires TLS unless plaintext SASL is explicitly enabled. Validation runs during ConfigMap rendering, including when an external Secret is used. ChangesStreaming SASL validation
Possibly related PRs
✨ Finishing Touches✨ Simplify code
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Completes the SASL fail-fast contract raised across the midaz #1741 reviews (Gandalf 6th review — three remaining REDs).
Problem
lib-streaming fails closed at bootstrap on an incomplete SASL config, but the chart rendered fine for:
STREAMING_TLS_ENABLED=false+STREAMING_SASL_ALLOW_PLAINTEXT=false.GSSAPI(only PLAIN, SCRAM-SHA-256, SCRAM-SHA-512 are supported).useExistingSecret=truebypassing username validation — the Secret template is skipped, soSTREAMING_SASL_USERNAME: ""shipped.Fix
Move the SASL contract validation into
streaming.env(the ConfigMap, which always renders — even with an external Secret). When a mechanism is set it must be supported, a username is required (a ConfigMap value, known at render regardless of the Secret source), and TLS must be on unless plaintext is explicitly opted into.Validation
useExistingSecretis covered)helm lintpasses. Negative assertions for the three scenarios (Ledger + CRM) land with the re-pin in #1741.